fix(mothership): keep a message the server never admitted instead of dropping it by waleedlatif1 · Pull Request #8674 · simstudioai/sim

waleedlatif1 · 2026-10-06T15:56:30Z

Summary

A send whose POST never got a response (offline, Wi-Fi drop, waking a laptop) reconnected to the stream it would have opened. That stream does not exist, so the 404 read as "finished": the turn finalized as a success and the message disappeared from the transcript with no error. A queued follow-up dispatched the same way was lost too.
A send refused with 409 because another turn held the chat (started in another tab, or one this tab lost track of) reconnected to that turn under the new message's bubble, then vanished when that turn finished.
Both now hand the message back under its id instead of consuming it:
- an unreachable send is held in the queue (not redispatched into the same failure) and goes out when the browser is back online, or when the user sends it;
- a send that found the chat busy waits in the queue behind the running turn, which the chat now shows as running, and goes out when it ends.
Reusing the id keeps a retry deduplicated if the server did admit the first attempt.

Type of Change

Bug fix

Testing

New DOM regressions in use-chat.dom.test.tsx (offline send held then sent on online under the same id; queued follow-up whose dispatch could not reach the server; send refused by another tab's turn goes out after it). All three fail on staging.
Browser E2E on a local Sim + worker stack with a scripted model: going offline, sending, coming back online now delivers the message and completes the turn (before: silently lost); a second tab that missed another tab's turn start now queues its message behind it (before: showed the other turn's answer under it, then lost it).

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

vercel · 2026-10-06T15:56:37Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated
docs	Skipped		Oct 6, 2026 7:08pm UTC

waleedlatif1 · 2026-10-06T15:56:42Z

@greptile

waleedlatif1 · 2026-10-06T15:56:44Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T15:56:48Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

5 issues found across 4 files

Confidence score: 3/5

In use-chat.ts, a busy-chat 409 is restored in a way that lets the active drain dispatch it again before the other turn finishes. Distinguish busy withdrawals from unmount withdrawals so the queue waits for the turn to finish.
In use-chat.ts, an unreachable send on a chatless surface is stored under a mount-specific key without the cross-surface handoff. Remounting before reconnection can strand the message under a dead key; preserve the handoff across remounts.
In use-chat.ts, held sends can stay blocked if the hook mounts after the online event or reconnects while a different chat is selected. Release held entries on mount when online and across chats when connectivity returns.
In types.ts, describe a POST with no response as an unconfirmed dispatch, since it may still have been admitted. Keep the same userMessageId on retry for deduplication.

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/stores/mothership-queue/types.ts">

<violation number="1" location="apps/sim/stores/mothership-queue/types.ts:17">
P3: A POST with no response may still have been admitted; `use-chat.ts` preserves the same `userMessageId` so its retry can deduplicate that attempt. Describe this as an unconfirmed/no-response dispatch rather than one that never reached the server.</violation>
</file>

<file name="apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts">

<violation number="1" location="apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts:4186">
P1: An unreachable send on a chatless surface is queued under the mount-specific pending key, but it no longer uses the cross-surface handoff. Remounting before reconnection strands that persisted message under a dead key; use a stable durable key or persist a held handoff that the next surface can recover.</violation>

<violation number="2" location="apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts:4806">
P1: A busy-chat 409 is restored with `retryRequired: false`, so the active queue drain immediately dispatches it again before the other turn finishes. Distinguish busy withdrawals from unmount withdrawals and keep this entry paused until `activeStreamId` clears.</violation>

<violation number="3" location="apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts:5020">
P2: This releases held sends only for the currently selected chat; reconnecting while viewing another chat leaves the original queue `retryRequired`, and returning online does not release it. Release held entries across chat keys and check `navigator.onLine` when a chat becomes active.</violation>

<violation number="4" location="apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts:5028">
P2: Held messages remain blocked when the hook mounts after the `online` event was already delivered. Release held entries on mount when `navigator.onLine` is true, in addition to listening for future events.</violation>
</file>

_{Turn on auto-fix | Re-trigger cubic}

greptile-apps · 2026-10-06T16:01:59Z

Confidence Score: 5/5

[Medium risk] Handles messages that fail to reach the server before going offline.

The PR appears safe to merge based on the reviewed changes and resolved previous findings.

Summary

The PR preserves sends that received no server response or were refused by a busy chat, retrying them under the same message ID. The latest changes add a session tombstone so a late queued-send restore does not recreate a deleted chat’s queue, with a regression test for that ordering.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Send or queued dispatch] --> B{Server response}
  B -->|No response| C[Hold under original message ID]
  B -->|Busy chat| D[Return behind running turn]
  C --> E[Release when online]
  D --> F[Dispatch when turn ends]
  E --> G[Retry with same ID]
  F --> G
  G --> H{Chat cleared?}
  H -->|Yes, late queue restore| I[Do not recreate queue]
  H -->|No| J[Continue queued send]

_{Reviews (6) · Last reviewed commit: "fix(mothership): don't recreate a delete..."}

waleedlatif1 · 2026-10-06T16:21:59Z

@greptile

waleedlatif1 · 2026-10-06T16:22:00Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T16:22:06Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

No issues found across 4 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{Turn on auto-fix | Re-trigger cubic}

waleedlatif1 · 2026-10-06T16:41:11Z

@greptile

waleedlatif1 · 2026-10-06T16:41:12Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T16:41:18Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

1 issue found across 4 files

Confidence score: 3/5

In use-chat.ts, an online event during the pending POST can leave the entry held after it’s added, even though the browser has reconnected. Track recovery during dispatch and release the entry after insertion when reconnect happened.

Prompt for AI agents (unresolved issues)


Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts">

<violation number="1" location="apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts:4210">
P2: An `online` event can fire while this POST is pending, before this entry is added, leaving it held after the browser reconnects. Track recovery during dispatch and release the entry after insertion when that event was missed.</violation>
</file>

_{Reply with feedback, questions, or to request a fix.

Fix all with cubic | Turn on auto-fix | Re-trigger cubic}

…dropping it Two sends were lost without an error: - A send whose POST got no response (offline, Wi-Fi drop, waking a laptop) reconnected to the stream it would have opened. That stream does not exist, so the 404 read as "finished", the turn finalized as a success, and the refetched transcript no longer held the message. A queued follow-up was lost the same way, since it had already left the queue. - A send refused with 409 because another turn held the chat (started in another tab, or one this surface lost track of) reconnected to that turn under the new message's bubble, then vanished when it finished. Both now hand the message back under its id. An unreachable send is held in the queue, so it is not redispatched into the same failure, and goes out when the browser is back online (or when the user sends it). A send that found the chat busy waits in the queue behind that turn, which the chat shows as running, and goes out when it ends. Reusing the id keeps a retry deduplicated if the server did admit the first attempt.

… remounts - A first message held offline on the new-chat page sat under that mount's queue key, which dies with the mount, so a reload or remount before the network returned stranded it. Held sends on a chatless surface now carry the surface they belong to, and the next chatless mount of that surface adopts them. - Held sends are released for every chat when the browser comes back online, and on mount when it already is, so a send held in a chat the user is not viewing (or one whose `online` event fired with no surface mounted) still goes out. - A send refused because the chat is busy is handed back only after the chat's running turn has been read, so the queue cannot redispatch it before that turn ends. A busy refusal that does not name the running turn no longer reads as a deduplicated send, which reconnected to a stream that never existed and lost the message.

…drain rules Releasing held sends on mount kicked the queue dispatcher directly, which skips the drain's guards. After a reload the chat history is not loaded yet, so a follow-up queued behind a still-running turn went out at once and was refused as busy. The release now only clears the hold; the drain effect, which waits for the history and for the running turn to end, sends a released head, and now also re-runs when the head's hold clears.

…s failing An `online` event can fire while the failing POST is still pending, so the release ran before the message was held and the message then waited for a release that had already happened. A send now notes whether the browser came back online while it was in flight, and if so goes back to the queue unheld, for the drain to send under its usual rules.

…not run" once Follow-ups from the #8673 review: the lease docs now name sign-out (`stopAllDesktopTools`) alongside the user's Stop as what cancels a desktop tool, and the stale-observation message no longer says it was not run twice.

waleedlatif1 · 2026-10-06T18:14:25Z

@greptile

waleedlatif1 · 2026-10-06T18:14:26Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T18:14:31Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

All reported issues were addressed across 7 files

_{Reply with feedback, questions, or to request a fix.

Fix all with cubic | Turn on auto-fix | Re-trigger cubic}

…switched chats A queued send had already left the queue when its POST failed, and a dispatch whose epoch changed meanwhile (the user switched chats) skipped restoring it, so the message was lost. A withdrawn send was never admitted, so it now goes back to its own chat's queue regardless of the epoch.

waleedlatif1 · 2026-10-06T18:29:22Z

@greptile

waleedlatif1 · 2026-10-06T18:29:23Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T18:29:28Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

All reported issues were addressed across 7 files

_{Reply with feedback, questions, or to request a fix.

Fix all with cubic | Turn on auto-fix | Re-trigger cubic}

…store A withdrawn send restored after its dispatch outlived a chat switch could land after the user deleted that chat, recreating a queue (and a message) for a conversation that no longer exists. Clearing a chat's queue now leaves a session tombstone that restores respect; a new enqueue for that key lifts it.

waleedlatif1 · 2026-10-06T19:08:16Z

@greptile

waleedlatif1 · 2026-10-06T19:08:17Z

@cubic-dev-ai review this PR

cubic-dev-ai · 2026-10-06T19:08:24Z

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai

No issues found across 7 files

Confidence score: 5/5

Automated review surfaced no issues in the provided summaries.
No files require special attention.

_{Turn on auto-fix | Re-trigger cubic}